su: pass all unknown options#9017
Conversation
|
I also recommend adding For example, su -- ls -iif no will make |
|
Already supported -- |
|
Add |
|
done |
|
doc also need ipdate |
|
done |
|
After PR #9017 was merged into the Magisk Alpha (starting with v29.x), several apps that previously worked without issues are now failing to function properly. This seems to be related to the recent changes in how su is handled, as reverting to an older Alpha build (pre-v29) or switching to the official Magisk release restores full functionality. Examples of affected apps: Valhalla Thor (Play Store) Google Shortcuts Launcher v5.5 (GitHub) Is there a recommended way to make these apps work with the latest magisk alpha (v29.x & above), or is this an intentional change that breaks compatibility? Looking forward to any guidance. |
|
I recommend that these apps use libsu instead of |
Thanks for the response. After inspecting the I've attached screenshots showing the relevant code references for both apps. This likely explains why their root functionality breaks on Magisk Alpha v29.x and above — due to the recent changes in how Is there any way to temporarily restore compatibility for such apps without needing them to update to |
|
Add quotes around the command, for example |
I've change 
|
|
not you, ask the app developer to fix it |
There was a problem hiding this comment.
Pull request overview
This pull request modifies the su command to remove built-in handling of -c (command) and -l (login) options and instead pass all unknown options directly to the remote shell. This allows users more flexibility to pass shell-specific options like --rcfile.
Changes:
- Removed
-cand-loption handling fromsu, allowing these and other unknown options to be passed to the shell - Changed the command structure from a single string to a vector of strings to properly handle arguments
- Updated help text and documentation to reflect the new behavior and option parsing rules
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| native/src/core/su/su.cpp | Main changes to option parsing, command building, and execution logic; removed -c and -l handlers; updated help text |
| native/src/core/su/daemon.rs | Updated SuRequest default to use empty command vector instead of empty string |
| native/src/core/lib.rs | Changed SuRequest struct to use Vec for command instead of separate shell and command strings; removed login field |
| native/src/core/su/connect.rs | Updated command logging to join vector elements with spaces |
| docs/tools.md | Updated documentation to match code changes in help text |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@vvb2060 is this behavior backwards compatible? I'd prefer to preserve the existing behavior, and only extend the functionality to forward unknown options to the inner shell. |
|
@topjohnwu difference: $ # before
$ su -c echo 'hello'
hello
$ # after
$ su -c echo 'hello'
$ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This changes the CLI API in a backwards incompatible way. I'll need more consideration before merging this PR. |
|
It is already compatible with the old commands. b915da2 |
-c, -l and more are now handled by remote shell.
http://www.mirbsd.org/htman/i386/man1/mksh.htm
This allows users to pass more options to the remote shell, such as rcfile.